support Asset and Op ExecutionContexts in standard execution path #17972
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
e01caed to
f724155
Compare
jamiedemaria
force-pushed
the
jamie/execution_path_with_two_contexts
branch
from
513ece0 to
8a1bc21
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
f724155 to
541fde7
Compare
jamiedemaria
force-pushed
the
jamie/execution_path_with_two_contexts
branch
from
8a1bc21 to
e64fb39
Compare
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit e64fb39. |
jamiedemaria
force-pushed
the
jamie/execution_path_with_two_contexts
branch
2 times, most recently
from
fd482bd to
18c5c92
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
a8f971d to
a45639e
Compare
jamiedemaria
force-pushed
the
jamie/execution_path_with_two_contexts
branch
from
a6d086b to
62b7ad6
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
a45639e to
115aabb
Compare
jamiedemaria
force-pushed
the
jamie/execution_path_with_two_contexts
branch
2 times, most recently
from
5043c25 to
770c04c
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
07736d6 to
815d4e3
Compare
jamiedemaria
force-pushed
the
jamie/execution_path_with_two_contexts
branch
from
770c04c to
4acf05c
Compare
This was referenced Nov 17, 2023
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
815d4e3 to
f172b7e
Compare
jamiedemaria
force-pushed
the
jamie/execution_path_with_two_contexts
branch
from
4acf05c to
4e90152
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
f172b7e to
5960e17
Compare
jamiedemaria
force-pushed
the
jamie/execution_path_with_two_contexts
branch
2 times, most recently
from
455bd9c to
63cf5b0
Compare
jamiedemaria
commented
Nov 22, 2023
| "_ExecutionProperties", | ||
| [ | ||
| ("step_description", PublicAttr[str]), | ||
| ("op_execution_context", PublicAttr["OpExecutionContext"]), |
There was a problem hiding this comment.
having to pass the OpExecutionContext here is kind of annoying, but the alternative is doing isinstance(context, AssetExecutionContext) checks in the core execution code so that we can access context.op_execution_context when the context is an AssetExecutionContext
There was a problem hiding this comment.
another option could be to move the functions needed from OpExecutionContext in the core execution path (has_events and consume_events) into the ExecutionProperties named tuple. I can look into that option as well. It might require passing the step execution context to this class, so might not be much better
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
cae085a to
3a8e919
Compare
jamiedemaria
force-pushed
the
jamie/execution_path_with_two_contexts
branch
from
63cf5b0 to
e5e974c
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
3a8e919 to
fa8509f
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
fc6cef4 to
358d4f4
Compare
jamiedemaria
force-pushed
the
jamie/execution_path_with_two_contexts
branch
from
58934d1 to
fe91874
Compare
This was referenced Jan 2, 2024
This was referenced Jan 12, 2024
This was referenced Jan 19, 2024
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
This PR is not strictly required to merge #17339. However, as we deprecate more methods, we will begin to deprecate context methods that are called in the main execution pathway.
This PR proposes adding a subobject to both AssetExecutionContext and OpExecutionContext that contains all methods/properties that need to be accessed in the execution code path.
How I Tested These Changes